Skip to content

Register encoding options when adding the service to the service collection#1575

Closed
hugohlln wants to merge 3 commits intoaws:masterfrom
hugohlln:master
Closed

Register encoding options when adding the service to the service collection#1575
hugohlln wants to merge 3 commits intoaws:masterfrom
hugohlln:master

Conversation

@hugohlln
Copy link

@hugohlln hugohlln commented Aug 31, 2023 •
edited by ashishdhingra
Loading

Issue:: #1574

Description of changes:
Can now register content type for transformation by passing options to the HostingOptions object

@brignolij
Copy link

brignolij commented Aug 31, 2023

Good idea, Very usefull for webp images with api gateway.

@ashishdhingra ashishdhingra changed the title Can register encoding options when adding the service to the service … Can register encoding options when adding the service to the service collection. Sep 5, 2023
@dscpinheiro dscpinheiro changed the title Can register encoding options when adding the service to the service collection. Register encoding options when adding the service to the service collection Sep 5, 2023
@dscpinheiro dscpinheiro requested a review from normj September 5, 2023 17:24
normj
normj requested changes Sep 8, 2023
View reviewed changes
Copy link
Member

@normj normj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I think the approach to solve the problem of configuring binary data is good. A few minor comments plus we also need some tests in the Amazon.Lambda.AspNetCoreServer.Test project.

@hugohlln
Copy link
Author

hugohlln commented Sep 8, 2023

@normj Thanks for your feedbacks

I just dit a new commit with following changes :

  • Add comments
  • Change the namespace of IEncodingOptions and EncodingOptions

For unit tests, what would be the best way to test the injection of IEncodingOptions ?

@ashishdhingra ashishdhingra added p2 This is a standard priority issue queued labels Sep 8, 2023
@ashishdhingra ashishdhingra requested a review from normj September 8, 2023 17:35
@CamileDahdah CamileDahdah self-requested a review September 27, 2023 17:11
@hugohlln
Copy link
Author

hugohlln commented Sep 28, 2023

@CamileDahdah

I just moved the registration of IEncodingOptions to LambdaRuntimeSupportServer level and removed redundant code from each AbstractAspNetCoreFunction<TREQUEST, TRESPONSE> implementations (APIGatewayHttpApiV2MinimalApi, APIGatewayRestApiMinimalApi and ApplicationLoadBalancerMinimalApi)

@ashishdhingra ashishdhingra removed the request for review from CamileDahdah July 3, 2024 22:01
@normj
Copy link
Member

normj commented Mar 18, 2026 •
edited
Loading

Closing as this feature was address in PR #2267 that exposed all of the extension points from Amazon.Lambda.AspNetCoreServer onto the HostingOptions including RegisterResponseContentEncodingForContentEncoding. That PR was released as part of version 1.10.0 of Amazon.Lambda.AspNetCoreServer.Hosting.

@hugohlln I appreciate your effort but it made sense finally address getting all of the extension points over.

@normj normj closed this Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@normj normj Awaiting requested review from normj

@philasmar philasmar Awaiting requested review from philasmar

1 more reviewer

@CamileDahdah CamileDahdah CamileDahdah left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug This issue is a bug. module/aspnetcore-support p2 This is a standard priority issue queued

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@hugohlln @brignolij @normj @CamileDahdah @ashishdhingra @hugohouillon